Skip to content

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented Jun 6, 2025

What do these changes do?

Aligns the public interface exposed by an aiohttp or FastAPI server for managing the lifecycle of a long running task.

Also delegates as much as possible of the implementation inside the common servicelib/long_running_tasks instead of the individual servicelib/aiohttp/long_running_tasks and servicelib/fastapi/long_running_tasks modules.

Removes some unused code

Related issue/s

How to test

Dev-ops

@GitHK GitHK self-assigned this Jun 6, 2025
@GitHK GitHK added this to the Bazinga! milestone Jun 6, 2025
@codecov
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

Attention: Patch coverage is 97.31183% with 5 lines in your changes missing coverage. Please review.

Project coverage is 87.96%. Comparing base (cf51daf) to head (37192b6).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7843      +/-   ##
==========================================
+ Coverage   87.89%   87.96%   +0.07%     
==========================================
  Files        1837     1640     -197     
  Lines       71032    66447    -4585     
  Branches     1219      912     -307     
==========================================
- Hits        62433    58450    -3983     
+ Misses       8256     7734     -522     
+ Partials      343      263      -80     
Flag Coverage Δ
integrationtests 64.20% <77.77%> (+0.04%) ⬆️
unittests 86.45% <97.31%> (-0.04%) ⬇️
Components Coverage Δ
api 76.84% <ø> (ø)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library 93.19% <100.00%> (+<0.01%) ⬆️
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration 69.92% <ø> (ø)
pkg_service_library 72.33% <97.14%> (+0.52%) ⬆️
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.05% <ø> (+0.05%) ⬆️
agent 96.29% <ø> (ø)
api_server 91.76% <ø> (ø)
autoscaling 96.03% <ø> (ø)
catalog 92.29% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 91.79% <ø> (ø)
datcore_adapter 97.94% <ø> (ø)
director 76.82% <ø> (ø)
director_v2 91.02% <100.00%> (-0.05%) ⬇️
dynamic_scheduler 96.69% <ø> (ø)
dynamic_sidecar 90.09% <100.00%> (+<0.01%) ⬆️
efs_guardian 89.65% <ø> (ø)
invitations 93.00% <ø> (ø)
payments 92.57% <ø> (ø)
resource_usage_tracker 88.98% <ø> (-0.11%) ⬇️
storage 87.46% <ø> (-0.08%) ⬇️
webclient ∅ <ø> (∅)
webserver 87.59% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf51daf...37192b6. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@GitHK GitHK changed the title Long running refactor 3 from2 ♻️ common public interface for managing long_running_tasks Jun 10, 2025

This comment was marked as outdated.

@GitHK GitHK changed the title ♻️ common public interface for managing long_running_tasks ♻️ common http API interface for long_running_tasks Jun 11, 2025
@pcrespov pcrespov requested a review from Copilot June 11, 2025 11:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the long running tasks API by unifying the HTTP interface for both aiohttp and FastAPI servers and delegating core implementation to a common servicelib module. Key changes include:

  • Standardizing configuration values to use timedelta instead of raw seconds.
  • Refactoring API endpoints and client/server modules to use shared http_endpoint_responses.
  • Removing deprecated or unused code and updating tests to align with the new interface.

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py Updated import paths for FastAPI client and periodic_task_result.
packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py Changed datetime import usage and updated configuration to use timedelta.
packages/service-library/src/servicelib/long_running_tasks/task.py Refactored TasksManager usage to support timedelta and replaced the close method with teardown.
packages/service-library/src/servicelib/long_running_tasks/models.py Introduced TaskGetWithoutHref and updated default factory for timestamps.
packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py Added helper functions to centralize HTTP endpoint responses.
packages/service-library/src/servicelib/long_running_tasks/constants.py Added new constants file for default poll and stale task intervals.
packages/service-library/src/servicelib/fastapi/long_running_tasks/* Updated client, server, routes, and error handlers to use the new configuration and common responses.
packages/service-library/src/servicelib/aiohttp/long_running_tasks/* Refactored server, client, routes, and dependencies to align with the new interface.
packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py Adjusted TaskGet hierarchy by splitting into TaskGetWithoutHref and extending TaskGet.
api/specs/web-server/* Updated response_model to Any for legacy endpoints.

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@sonarqubecloud
Copy link

@GitHK GitHK added the 🤖-automerge marks PR as ready to be merged for Mergify label Jun 12, 2025
@GitHK
Copy link
Contributor Author

GitHK commented Jun 12, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jun 12, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at c7b7878

@mergify mergify bot merged commit c7b7878 into ITISFoundation:master Jun 12, 2025
298 of 308 checks passed
@GitHK GitHK deleted the long-running-refactor-3-from2 branch June 16, 2025 13:54
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants